Skip to content

Introduce AbstractAsyncSelector - #6097

Merged
jrhee17 merged 3 commits into
line:mainfrom
jrhee17:refactor/async-selector
Mar 26, 2025
Merged

Introduce AbstractAsyncSelector#6097
jrhee17 merged 3 commits into
line:mainfrom
jrhee17:refactor/async-selector

Conversation

@jrhee17

@jrhee17 jrhee17 commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

Motivation:

AbstractEndpointSelector is a useful abstract which allows users to select and Endpoint asynchronously only by implementing AbstractEndpointSelector#selectNow. However, the current implementation has limitations in that 1) Only Endpoint can be selected 2) It is difficult to customize the timing when the pending futures must be reconsidered for completion.

While working on the introduction of an XdsPreprocessor #6096, I found it is useful to be able to select a value asynchronously in the following locations:

  1. When a request is executed, the request must wait until a ListenerSnapshot is fetched

    public O execute(PreClient<I, O> delegate, PreClientRequestContext ctx, I req) throws Exception {

  2. The XdsLoadBalancer which selects an endpoint must be refreshed either when the encompassed EndpointGroup is updated or when the local cluster is updated.

For ease of implementation and maintainability, I propose that AbstractAsyncSelector is abstracted out of AbstractEndpointSelector. Since it is unclear whether this abstraction is useful for other situations, the class is located in the internal package.

Modifications:

  • Abstracted out selection related logic from AbstractEndpointSelector to AbstractAsyncSelector
    • Now AbstractEndpointSelector contains an internal AbstractAsyncSelector implementation
  • Moved SelectionTimeoutTest#select_unlimited to the internal package since it relies on visibility of ListenableFuture#timeoutFuture

Result:

  • Users can implement async selection logic easily

@jrhee17 jrhee17 added this to the 1.33.0 milestone Feb 14, 2025
@jrhee17
jrhee17 marked this pull request as ready for review February 14, 2025 09:05

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@github-actions github-actions Bot added the Stale label Mar 22, 2025
@jrhee17
jrhee17 merged commit 611c1fb into line:main Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants